Defend against attempts to bypass JVM serial proxy #522
Conversation
Also add serialVersionUID for exception classes. This ensures that unrelated changes in these exception classes don't affect the serialized form. The serialVersionUIDs are from 7380325. After running ./gradlew publishToMavenLocal, they were obtained with the serialver tool [1]: $ serialver -classpath .m2/repository/org/jetbrains/kotlinx/kotlinx-datetime-jvm/0.6.2-SNAPSHOT/kotlinx-datetime-jvm-0.6.2-SNAPSHOT.jar:kotlin-stdlib-2.1.0.jar kotlinx.datetime.LocalDate kotlinx.datetime.LocalDateTime kotlinx.datetime.LocalTime kotlinx.datetime.UtcOffset kotlinx.datetime.DateTimeArithmeticException kotlinx.datetime.IllegalTimeZoneException kotlinx.datetime.DateTimeFormatException kotlinx.datetime.internal.format.parser.ParseException kotlinx.datetime.LocalDate: private static final long serialVersionUID = 7026816023079564263L; kotlinx.datetime.LocalDateTime: private static final long serialVersionUID = -4261744960416354711L; kotlinx.datetime.LocalTime: private static final long serialVersionUID = -352249606036216323L; kotlinx.datetime.UtcOffset: private static final long serialVersionUID = -6636773355667981618L; kotlinx.datetime.DateTimeArithmeticException: private static final long serialVersionUID = -3207806170214997982L; kotlinx.datetime.IllegalTimeZoneException: private static final long serialVersionUID = 1159315966274264801L; kotlinx.datetime.DateTimeFormatException: private static final long serialVersionUID = 4231196759387994100L; kotlinx.datetime.internal.format.parser.ParseException: private static final long serialVersionUID = 5691186997393344103L; [1] https://docs.oracle.com/en/java/javase/21/docs/specs/man/serialver.html
core/jvm/src/LocalDate.kt
Outdated
|
|
||
| // even though this class uses writeReplace (so serialVersionUID is not needed for a stable serialized form), a | ||
| // stable serialVersionUID means exceptions caused by deserialization of malicious streams will be consistent | ||
| // (InvalidClassException vs. InvalidObjectException, see MaliciousJvmSerializationTest) |
There was a problem hiding this comment.
Is this really needed? As soon as some runtime serialization exception is thrown on a malicious stream, does it really matter which one?
There was a problem hiding this comment.
I think you can argue both ways. But the change is minimal (only this one additional field), so it could just be kept this way. I have no strong opinion here. I can remove it if you think it's not important to ensure this behavior.
There was a problem hiding this comment.
I realized that explicitly declaring the serialVersionUID is needed to properly do the tests in MaliciousJvmSerializationTest. The malicious serial streams in that test are designed to bypass the proxy given no defensive readObject method. But that only works if they have the correct serialVersionUID in them. So they would have to be regenerated each time the serialVersionUID changes (which e.g. happened for LocalDate in d15ec21 due to new methods in that class, it is now -9141474295923275006L). Otherwise we would just test that ObjectInputStream detects and rejects invalid serialVersionUIDs.
There was a problem hiding this comment.
I've updated the comment to say it's useful for testing: 8811dcc
|
For 89fd818 serialver now outputs this ( I won't update it here though. The actual value doesn't matter, non-malicious serialization will not use it since it goes through |
Also add
serialVersionUIDfor exception classes. This ensures that unrelated changes in these exception classes don't affect the serialized form.The
serialVersionUIDs are from 7380325. After running./gradlew publishToMavenLocal, they were obtained with the serialver tool: